feat(rollup-relayer): support codecv8 #1681
Conversation
|
Warning Rate limit exceeded@colinlyguo has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 7 minutes and 26 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (4)
📒 Files selected for processing (8)
""" WalkthroughThis change updates dependency versions for several modules, increments the main version tag, adds support for codec version 8 in batch processing logic, and introduces a new configuration field in the proposer tool genesis JSON. Method signatures and logic in the relayer are adjusted to handle codec version 8 alongside version 7. Configuration and proposer components are enhanced to enforce a new uncompressed batch size limit with related metrics and tests updated accordingly. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Layer2Relayer
participant DB
User->>Layer2Relayer: ProcessPendingBatches()
Layer2Relayer->>Layer2Relayer: Determine codec version (V7 or V8)
alt Codec V7 or V8
Layer2Relayer->>Layer2Relayer: constructCommitBatchPayloadCodecV7()
Layer2Relayer->>Layer2Relayer: contextIDFromBatches(codecVersion, batches)
else Other versions
Layer2Relayer->>Layer2Relayer: Handle accordingly
end
Layer2Relayer->>DB: SendTransaction(contextID)
DB-->>Layer2Relayer: Transaction result
Possibly related PRs
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
469-475: Method name & log message now misleading for V8 supportThe switch correctly re-uses
constructCommitBatchPayloadCodecV7for both V7 and V8, but
- the helper’s name still ends in “CodecV7”;
- the error log hard-codes “…payload for V7”.
This will confuse future maintainers when troubleshooting V8 flows. Rename the helper (e.g.
constructCommitBatchPayloadCodecV7V8) or introduce a thin wrapper for V8, and update the log message to reflect the actual codecVersion.
🧹 Nitpick comments (1)
rollup/proposer-tool-config.json (1)
4-8: Validate increased chunk proposer limits
Raisingmax_block_num_per_chunkto1000andmax_l2_gas_per_chunkto200000000000can stress memory, database, and network layers. Confirm that the infrastructure and upstream components can handle these new limits without performance regressions.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (4)
bridge-history-api/go.sumis excluded by!**/*.sumcoordinator/go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumrollup/go.sumis excluded by!**/*.sum
📒 Files selected for processing (11)
bridge-history-api/go.mod(1 hunks)common/version/version.go(1 hunks)coordinator/go.mod(1 hunks)coordinator/internal/logic/provertask/batch_prover_task.go(1 hunks)coordinator/internal/types/prover.go(1 hunks)rollup/docker-compose-proposer-tool.yml(1 hunks)rollup/go.mod(1 hunks)rollup/internal/controller/relayer/l2_relayer.go(4 hunks)rollup/internal/controller/watcher/proposer_tool.go(1 hunks)rollup/proposer-tool-config.json(1 hunks)rollup/proposer-tool-genesis.json(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
rollup/internal/orm/batch.go (2)
Batch(22-72)Batch(80-82)
🔇 Additional comments (10)
common/version/version.go (1)
8-8: Version tag bumped correctly.The patch version was incremented to
v4.5.22in line with the updatedda-codecandgo-ethereumdependencies across modules.rollup/internal/controller/watcher/proposer_tool.go (1)
1-162: No logic changes detected.This PR only reorders the import of
scroll-tech/database/migrate; functionality remains unchanged.rollup/go.mod (1)
14-15: Ensure consistency of dependency versions across modules.Please verify that the updated
da-codecandgo-ethereumcommit hashes here match exactly incoordinator/go.modandbridge-history-api/go.modto avoid build mismatches.Run:
#!/bin/bash rg "github.com/scroll-tech/da-codec v0.1.3-0.20250616065246-17bf3e4990cd" -l rg "github.com/scroll-tech/go-ethereum v1.10.14-0.20250611141528-cf3d22ef8707" -lcoordinator/go.mod (1)
12-13: Align dependency versions with other modules.The
da-codecandgo-ethereumversions were bumped here; confirm they are identical to those inrollup/go.modandbridge-history-api/go.modto maintain cross-module compatibility.bridge-history-api/go.mod (2)
13-14: Verify updated require directives.The
da-codecandgo-ethereumversions have been upgraded; ensure these commits align with those in therollupandcoordinatormodules.
21-21: Confirm hotfix coverage in replace directive.The replace directive for
go-ethereumpoints to commit20250616074821-cba908db071dwith a note about a header hash hotfix. Please verify that this commit indeed contains the intended fix.coordinator/internal/types/prover.go (1)
3-7: No functional changes detected
The blank line added between the"fmt"and"scroll-tech/common/types/message"imports is purely stylistic and does not affect behavior.rollup/docker-compose-proposer-tool.yml (1)
30-32: Verify updated--start-l2-blockvalue
Ensure that the new block number"14907015"correctly aligns with the intended activation point for CODEC V8 support and that no downstream tooling assumes the old start height.rollup/proposer-tool-genesis.json (1)
8-11: EnsurefeynmanTimefield is utilized
The new"feynmanTime": 0field has been added to the genesis config. Verify that the proposer‐tool code actually reads and applies this parameter, otherwise it may be silently ignored.coordinator/internal/logic/provertask/batch_prover_task.go (1)
267-272: Extend batch codec support to V8
Incorporatingencoding.CodecV8alongside existing versions is correct. Please ensure you have added or updated unit/integration tests to cover the new V8 decoding path and validate backward compatibility.
colinlyguo
force-pushed
the
feat-rollup-relayer-support-codecv8
branch
from
30153c1 to
377e8bb
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1681 +/- ##
===========================================
- Coverage 40.15% 40.11% -0.05%
===========================================
Files 234 234
Lines 18614 18618 +4
===========================================
- Hits 7474 7468 -6
- Misses 10409 10423 +14
+ Partials 731 727 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
468-476: Minor log message mismatch after Codec V8 support
The error log inside the new switch block is hard-coded to “for V7”, which is misleading whencodecVersionis V8.- log.Error("failed to construct constructCommitBatchPayloadCodecV7 payload for V7", "codecVersion", codecVersion, ... + log.Error("failed to construct commitBatch payload", "codecVersion", codecVersion, ...Purely cosmetic, but worth adjusting to avoid confusion when debugging.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
rollup/internal/controller/relayer/l2_relayer.go(4 hunks)
🧰 Additional context used
🪛 golangci-lint (1.64.8)
rollup/internal/controller/relayer/l2_relayer.go
525-525: undefined: dbBatchWithChunksAndParent
(typecheck)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: check
- GitHub Check: tests
- GitHub Check: test
- GitHub Check: check
- GitHub Check: tests
🔇 Additional comments (1)
rollup/internal/controller/relayer/l2_relayer.go (1)
534-537: Future-proof parsing LGTM
The generalizedbatchHashesFromContextIDcorrectly handles anyv<N>-…prefix and gracefully falls back. Nice improvement over the old fixed-version check.
colinlyguo
force-pushed
the
feat-rollup-relayer-support-codecv8
branch
from
e5a75c7 to
d274123
Compare
colinlyguo
force-pushed
the
feat-rollup-relayer-support-codecv8
branch
from
10a5802 to
c3eafe5
Compare
colinlyguo
force-pushed
the
feat-rollup-relayer-support-codecv8
branch
2 times, most recently
from
64b583d to
5c5ab15
Compare
colinlyguo
force-pushed
the
feat-rollup-relayer-support-codecv8
branch
from
5c5ab15 to
f60c290
Compare
Purpose or design rationale of this PR
Enable rollup-relayer to support codecv8 and update bridge-history's da-codec commit for batch hash validation compatibility.
PR title
Your PR title must follow conventional commits (as we are doing squash merge for each PR), so it must start with one of the following types:
Deployment tag versioning
Has
tagincommon/version.gobeen updated or have you addedbump-versionlabel to this PR?Breaking change label
Does this PR have the
breaking-changelabel?Summary by CodeRabbit
New Features
Improvements
Tests
Chores